Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase and @matan-starkware)

crates/starknet_l1_provider/src/l1_scraper.rs line 55 at r1 (raw file):

            self.fetch_events().await?;
        }
    }

Is this the canonical run implementation in our node?

Code quote:

    async fn _run(&mut self) -> L1ScraperResult<(), B> {
        loop {
            sleep(self.config.polling_interval).await;
            // TODO: retry.
            self.fetch_events().await?;
        }
    }

crates/starknet_l1_provider/src/l1_scraper.rs line 60 at r1 (raw file):

#[derive(Clone, Debug, Serialize, Deserialize, Validate, PartialEq)]
pub struct L1ScraperConfig {
    // TODO: make this config into trait.

WDYM? Why?

Code quote:

// TODO: make this config into trait.

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase and @matan-starkware)

Reviewed 4 of 7 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @giladchase)

-- commits line 4 at r2:
We don't want these to be configurable?

Code quote:

  - base layer constant file will hold the identifiers of Starknet Events.

crates/starknet_l1_provider/src/l1_scraper_tests.rs line 0 at r2 (raw file):
Why add an empty file?

Reviewable status: 5 of 8 files reviewed, 3 unresolved discussions (waiting on @elintul and @matan-starkware)

-- commits line 4 at r2:

I might be misunderstanding you, but AFAIK event identifiers (in Ethereum) are fixed, they are always:
keccak(<signature_of_event>).
The identifier will only change if the signature changes, and this updates the const during compilation time (since the consts are generated from sol!).

crates/starknet_l1_provider/src/l1_scraper.rs line 55 at r1 (raw file):

I haven't found any similar run use-case in our code. We currently only have loop-and-select! style runs, without any loop-do-sleep-awake loops.

With that said, this is a very basic implementation, I'll extend it soon once the pressure's off, unless you have stuff in mind now?

crates/starknet_l1_provider/src/l1_scraper.rs line 60 at r1 (raw file):

Seems to be refactoring leftovers, doesn't make sense here, removed.

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase and @matan-starkware)

crates/starknet_l1_provider/src/l1_scraper.rs line 55 at r1 (raw file):

Nope, sounds good.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase and @matan-starkware)

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matan-starkware)

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matan-starkware)

Reviewed 1 of 7 files at r1, 2 of 3 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @giladchase)

-- commits line 4 at r2:

I understand a given event has a fixed identifier, I was wondering if the set of identifiers we want to track may change over time or with tests.

Ok thanks for explaining about them being generated by sol!. I see in your later PR pub const LOG_MESSAGE_TO_L2_EVENT_IDENTIFIER: &str = Starknet::LogMessageToL2::SIGNATURE; So my idea was infeasible

Reviewable status: 5 of 8 files reviewed, all discussions resolved (waiting on @elintul and @matan-starkware)

-- commits line 4 at r2:

I understand a given event has a fixed identifier, I was wondering if the set of identifiers we want to track may change over time or with tests.

For sure, that is covered by the tracked_event_identifiers field which is passed to the provider in its constructor.

Reviewed 3 of 7 files at r1, 1 of 2 files at r2, 1 of 3 files at r3, 3 of 3 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @giladchase)